Fix: Embed Block: Match HTML in the editor and frontend #65478

hbhalodia · 2024-09-19T05:34:21Z

What?

Fixes - Embed Block: Match HTML in the editor and frontend #65179

Why?

To match the exact HTML for frontend and backend as per - Embed Block: Match HTML in the editor and frontend #65179

How?

PR removes the block wrapper div, instead adds figure as a wrapper, so it matches with frontend markup.

Testing Instructions

Open any post/page.
Add embed block.
Add URL to be embeded, check for frontend and backend markup.
It should be similar.

Testing Instructions for Keyboard

None.

Screenshots or screencast

Backend

<figure tabindex="0" class="block-editor-block-list__block wp-block is-selected wp-embed-aspect-16-9 wp-has-aspect-ratio wp-block-embed" id="block-8d19fac7-16f5-44ac-8ae0-6630fd12dc9d" role="document" aria-label="Block: YouTube Embed" data-block="8d19fac7-16f5-44ac-8ae0-6630fd12dc9d" data-type="core/embed" data-title="YouTube Embed">
    <div class="wp-block-embed__wrapper">
        <iframe title="Embedded content from www.youtube.com" class="components-sandbox" sandbox="allow-scripts allow-same-origin allow-presentation" width="462" height="260">
        </iframe>
    </div>
</figure>

Frontend

<figure class="wp-block-embed is-type-video is-provider-youtube wp-block-embed-youtube wp-embed-aspect-16-9 wp-has-aspect-ratio">
    <div class="wp-block-embed__wrapper">
        <iframe title="Alexandre Corbeil on Gaining New Skills at Trebas Institute Toronto" width="500" height="281" src="https://www.youtube.com/embed/BOM34PANOHM?feature=oembed" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" referrerpolicy="strict-origin-when-cross-origin" allowfullscreen="">
    </iframe>
    </div>
</figure>

github-actions · 2024-09-19T05:34:34Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

t-hamano

Thanks for the PR!

There are two things that need to be addressed in order for this PR to move forward.

Fix E2E tests

The figure tag applies to the block itself, not inside it. Therefore, currenEmbedBlock.locator('figure') can no longer find the figure element.

The following changes are required:

diff --git a/test/e2e/specs/editor/various/embedding.spec.js b/test/e2e/specs/editor/various/embedding.spec.js
index fb9746dce5..fe488f9130 100644
--- a/test/e2e/specs/editor/various/embedding.spec.js
+++ b/test/e2e/specs/editor/various/embedding.spec.js
@@ -134,15 +134,15 @@ test.describe( 'Embedding content', () => {
                        'https://developer.wordpress.org/block-editor/reference-guides/block-api/block-attributes/'
                );
                await expect(
-                       currenEmbedBlock.locator( 'figure' ),
+                       currenEmbedBlock,
                        'WordPress valid content. Should render valid figure element.'
-               ).toHaveClass( 'wp-block-embed' );
+               ).toHaveClass( /wp-block-embed/ );
 
                await embedUtils.insertEmbed(
                        'https://www.youtube.com/watch?v=lXMskKTw3Bc'
                );
                await expect(
-                       currenEmbedBlock.locator( 'figure' ),
+                       currenEmbedBlock,
                        'Video content. Should render valid figure element, and include the aspect ratio class.'
                ).toHaveClass( /wp-embed-aspect-16-9/ );

Restore `is-type-video` CSS class

On the frontend, the figure tag may have classes like is-type-video applied, but not in the editor. We need to restore the removed CSS to the block wrapper.

hbhalodia · 2024-09-23T05:05:44Z

Thank You, @t-hamano for review. I would address the requested feedbacks.

hbhalodia · 2024-09-23T06:36:51Z

Hi @t-hamano, On adding the className to the block wrapper, some functionalities are breaking in the editor, for example, If I do not add the class, then if you click on the video, you can see the block controls, and you can update it, but on adding the class, block controls are not visible, and you would not able to update anything. Here is the screencast for the same.

issue-embed.1.mov

t-hamano · 2024-09-23T12:33:30Z

We cannot add the className prop directly to the figure element.

The blockProps itself contains classNames, but if we add the className prop after blockProps, the className from blockProps will be overwritten and the block will not work properly.

We will probably need to add the class name via useBlockProps, like so:

const blockProps = useBlockProps( {
	className: clsx( {
		'is-type-video': 'video' === type,
	} ),
} );

hbhalodia · 2024-09-24T04:40:09Z

Hi @t-hamano, I have added the requested changes, and added the fix by changing the blockprops.className, this is because, className and type are added after the blockProps is being added, hence we cannot directly add there, so need to update the className in a way.

Thank You,

t-hamano · 2024-09-26T13:40:55Z

@hbhalodia Thanks for the update!

I've tried several providers, or updating additional CSS classes, etc. and everything seems to work fine.

Could you rebase this PR? It should fix the E2E test failure.

This might be a matter of preference, but class names might be better for an ad-hoc approach:

diff --git a/packages/block-library/src/embed/edit.js b/packages/block-library/src/embed/edit.js
index b69fc75cef..17f6261751 100644
--- a/packages/block-library/src/embed/edit.js
+++ b/packages/block-library/src/embed/edit.js
@@ -251,9 +251,6 @@ const EmbedEdit = ( props ) => {
                className: classFromPreview,
        } = getMergedAttributes();
        const className = clsx( classFromPreview, props.className );
-       blockProps.className = clsx( blockProps.className, className, {
-               'is-type-video': 'video' === type,
-       } );
 
        return (
                <>
@@ -265,7 +262,12 @@ const EmbedEdit = ( props ) => {
                                toggleResponsive={ toggleResponsive }
                                switchBackToURLInput={ () => setIsEditingURL( true ) }
                        />
-                       <figure { ...blockProps }>
+                       <figure
+                               { ...blockProps }
+                               className={ clsx( blockProps.className, className, {
+                                       'is-type-video': 'video' === type,
+                               } ) }
+                       >
                                <EmbedPreview
                                        preview={ preview }
                                        previewable={ previewable }

Similar approaches can be found here and here.

While testing this PR, I noticed that there were some CSS classes missing from the editor. It would be nice to address this in a follow-up:
- wp-block-embed-{variationName} (example: wp-block-embed-youtube)
- is-provider-{providerName} (example: is-provider-youtube)
- is-type-rich (For example, when you embed Twitter, this CSS class is added to the front-end:)

hbhalodia · 2024-10-07T04:20:07Z

Hi @t-hamano, thanks for the feedback. I would update the PR with requested changes.

hbhalodia · 2024-10-07T04:46:49Z

Hi @t-hamano, I have updated the PR with the requested changes. Can you please review the same.
Thank You,

t-hamano

LGTM! Thanks for the update 👍

Add the fix for embed to resemeble exact markup as frontend and backend

ca3988d

hbhalodia requested a review from ajitbohra as a code owner September 19, 2024 05:34

hbhalodia mentioned this pull request Sep 19, 2024

Embed Block: Match HTML in the editor and frontend #65179

Closed

t-hamano added [Type] Enhancement A suggestion for improvement. [Block] Embed Affects the Embed Block labels Sep 22, 2024

t-hamano self-requested a review September 22, 2024 13:48

t-hamano reviewed Sep 22, 2024

View reviewed changes

hbhalodia added 2 commits September 23, 2024 10:36

Merge branch 'trunk' into fix/issue-65179

a0a3a81

Fix e2e test for updated embed block

2ed84ee

Add the blockprops className to use the newClassname

3578238

hbhalodia requested a review from t-hamano September 24, 2024 04:38

hbhalodia added 4 commits October 7, 2024 09:53

Merge branch 'trunk' into fix/issue-65179

6e7a4ed

Fix the code syntax for the same

a57f134

Add the missing class on editor for the embed block

e499afe

Add is-type-<typename> class to the editor embed block

5e7c8bd

t-hamano approved these changes Oct 8, 2024

View reviewed changes

t-hamano merged commit d6a4c72 into WordPress:trunk Oct 8, 2024
62 checks passed

github-actions bot added this to the Gutenberg 19.5 milestone Oct 8, 2024

hbhalodia linked an issue Oct 15, 2024 that may be closed by this pull request

Embed Block: Match HTML in the editor and frontend #65179

Closed

WontonSam mentioned this pull request Oct 27, 2024

[Snyk] Upgrade @wordpress/e2e-test-utils from 7.2.1 to 11.9.0 WontonSam/chicommerce#963

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Embed Block: Match HTML in the editor and frontend #65478

Fix: Embed Block: Match HTML in the editor and frontend #65478

hbhalodia commented Sep 19, 2024

github-actions bot commented Sep 19, 2024 •

edited

Loading

t-hamano left a comment

hbhalodia commented Sep 23, 2024

hbhalodia commented Sep 23, 2024

t-hamano commented Sep 23, 2024

hbhalodia commented Sep 24, 2024

t-hamano commented Sep 26, 2024

hbhalodia commented Oct 7, 2024

hbhalodia commented Oct 7, 2024

t-hamano left a comment

Fix: Embed Block: Match HTML in the editor and frontend #65478

Fix: Embed Block: Match HTML in the editor and frontend #65478

Conversation

hbhalodia commented Sep 19, 2024

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

Backend

Frontend

github-actions bot commented Sep 19, 2024 • edited Loading

t-hamano left a comment

Choose a reason for hiding this comment

Fix E2E tests

Restore is-type-video CSS class

hbhalodia commented Sep 23, 2024

hbhalodia commented Sep 23, 2024

t-hamano commented Sep 23, 2024

hbhalodia commented Sep 24, 2024

t-hamano commented Sep 26, 2024

hbhalodia commented Oct 7, 2024

hbhalodia commented Oct 7, 2024

t-hamano left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 19, 2024 •

edited

Loading

Restore `is-type-video` CSS class